ποΈGitΠ―ΡΠ°ποΈ
Commit 5235b8fc51eb5f46d453effb2deec00bcc6abdbb
Parents : 2d63db8
Author : James Rich <2199651+jamesarich@users.noreply.github.com>
Signature : Signature validation error
Date : 2026-07-27T13:53:54-05:00
Committer : GitHub <noreply@github.com>
Date : 2026-07-27T18:53:54Z
fix(logging): stop reporting expected conditions as crashes (#6470)
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Changes
16 files changed, 598 insertions(+), 37 deletions(-)
Diff
diff --git a/.skills/code-review/SKILL.md b/.skills/code-review/SKILL.md
index e499058488..e58934add9 100644
--- a/.skills/code-review/SKILL.md
+++ b/.skills/code-review/SKILL.md
@@ -55,7 +55,24 @@ When reviewing code, meticulously verify the following categories. Flag any devi
- [ ] **Libraries:** Verify usage of `Turbine` for Flow testing, `Kotest` for property-based testing, and `Mokkery` for mocking.
- [ ] **Robolectric Configuration:** Check that Compose UI tests running via Robolectric on JVM are pinned to `@Config(sdk = [34])` to prevent SDK 35 compatibility issues.
-### 8. ProGuard / R8 Rules
+### 8. Logging & Crash Reporting
+Kermit is the only logging API, and on the **google** flavor its writers fan every call out to **both** Firebase Crashlytics and Datadog RUM (`androidApp/src/google/.../GooglePlatformAnalytics.kt`). Log level is therefore a *reporting* decision, not just a verbosity one.
+
+**The rule: `Logger.e` means "a defect someone can fix". Everything else is `Logger.w` or below.**
+
+- [ ] **Severity gates reporting:** `Severity.Error`/`Assert` become a Crashlytics non-fatal (`shouldReportAsException`, which exempts `CancellationException` and any `ExpectedCondition` in the cause chain) **and** a Datadog RUM error (`shouldDowngradeForDatadog`, which exempts only `ExpectedCondition`). `Warn` and below never report in either sink, with no exceptions. Attaching a throwable at warn level is free and keeps the stack trace in the logs, so demoting costs nothing.
+- [ ] **Don't "unify" the two cancellation rules.** Crashlytics drops `CancellationException` because it is a crash-triage tool; Datadog keeps it because a cancellation logged at *error* means a call site swallowed it instead of rethrowing β broken structured concurrency, and a real bug. That asymmetry is the detector that found #6468. Likewise, neither rule unwraps the cause chain for cancellation: coroutine machinery attaches cancellations as the cause of unrelated genuine failures, and unwrapping would silently drop those reports.
+- [ ] **`Logger.e` with no throwable still reports.** Crashlytics synthesises an `Exception(message)`; Datadog raises a RUM error from the level alone. `Logger.e { "β¦" }` is *not* a cheap log line.
+- [ ] **Expected conditions must not be reported.** Bluetooth off, a permission not granted, location services off, a deliberate disconnect, a peer/broker protocol violation, a handled retry, a guard that is doing its job β these are environment states, not bugs. Reporting them buries real regressions during release triage.
+- [ ] **Use the `ExpectedCondition` seam** (`core/common/src/commonMain/.../log/ExpectedCondition.kt`):
+ - Exception type that *only ever* means "the environment said no" β implement `ExpectedCondition` and give it a stable, low-cardinality `expectedConditionLabel` (e.g. `ble-scan-bluetooth-disabled`). `BleScanStartException` is the reference example.
+ - Exception type shared between expected and genuine failures β leave the type alone and log that call site at `Logger.w`.
+ - Both sinks consult `shouldReportAsException(severity, throwable)`, so an `ExpectedCondition` is suppressed even if some call site logs it at error. Treat that as a backstop, not a licence to log expected states at error.
+- [ ] **Prefer a rate over an exception.** For conditions worth *watching* but not *fixing* (watchdog fired, reconnect attempt failed), emit a warn log with a stable label and track its rate in the log backend. Do not manufacture a throwable just to get a stack trace.
+- [ ] **Third-party log bridges:** adapters that forward another library's logs into Kermit must downgrade that library's "error" level β its errors are usually operational. See `core/ble/.../KermitLogEngine.kt` (Kable).
+- [ ] **New `Logger.e` in a PR:** ask what the on-call engineer would *do* about it. If the answer is "nothing, that's just the user's phone", it is a `Logger.w`.
+
+### 9. ProGuard / R8 Rules
- [ ] **New Dependencies:** If a new reflection-heavy dependency is added (DI, serialization, JNI, ServiceLoader), verify keep rules exist in **both** `androidApp/proguard-rules.pro` (R8) and `desktopApp/proguard-rules.pro` (ProGuard). The two files must stay aligned.
- [ ] **Release Smoke-Test:** For dependency or ProGuard rule changes, verify `assembleRelease` and `./gradlew :desktopApp:runRelease` succeed.
diff --git a/.skills/compose-ui/strings-index.txt b/.skills/compose-ui/strings-index.txt
index 33ae74fb9b..e63d403ff6 100644
--- a/.skills/compose-ui/strings-index.txt
+++ b/.skills/compose-ui/strings-index.txt
@@ -131,6 +131,7 @@ bluetooth_feature_config_description
bluetooth_feature_discovery
bluetooth_feature_discovery_description
bluetooth_permission
+bluetooth_scan_location_services_disabled
bluetooth_scan_missing_permission
bluetooth_scan_start_failed
bluetooth_scan_too_frequent
diff --git a/androidApp/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt b/androidApp/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt
index 85f48aa0fb..32989f3f5c 100644
--- a/androidApp/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt
+++ b/androidApp/src/google/kotlin/org/meshtastic/app/analytics/GooglePlatformAnalytics.kt
@@ -52,11 +52,12 @@ import com.google.firebase.crashlytics.crashlytics
import com.google.firebase.crashlytics.setCustomKeys
import com.google.firebase.initialize
import io.opentelemetry.api.GlobalOpenTelemetry
-import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import org.koin.core.annotation.Single
import org.meshtastic.app.BuildConfig
+import org.meshtastic.core.common.log.shouldDowngradeForDatadog
+import org.meshtastic.core.common.log.shouldReportAsException
import org.meshtastic.core.repository.AnalyticsPrefs
import org.meshtastic.core.repository.DataPair
import org.meshtastic.core.repository.PlatformAnalytics
@@ -301,21 +302,18 @@ class GooglePlatformAnalytics(private val context: Context, private val analytic
// Add the log to the Crashlytics log buffer so it appears in reports
Firebase.crashlytics.log("$severity/$tag: $message")
- // Filter out normal coroutine cancellations
- if (throwable is CancellationException) return
+ // Cancellations and expected conditions stay breadcrumbs only β see shouldReportAsException.
+ if (!shouldReportAsException(severity, throwable)) return
- // Only record non-fatal exceptions for actual Errors (Severity.Error or Severity.Assert)
- if (severity >= Severity.Error) {
- if (throwable != null) {
- Firebase.crashlytics.recordException(throwable)
- } else {
- Firebase.crashlytics.setCustomKeys {
- key(KEY_PRIORITY, severity.ordinal)
- key(KEY_TAG, tag)
- key(KEY_MESSAGE, message)
- }
- Firebase.crashlytics.recordException(Exception(message))
+ if (throwable != null) {
+ Firebase.crashlytics.recordException(throwable)
+ } else {
+ Firebase.crashlytics.setCustomKeys {
+ key(KEY_PRIORITY, severity.ordinal)
+ key(KEY_TAG, tag)
+ key(KEY_MESSAGE, message)
}
+ Firebase.crashlytics.recordException(Exception(message))
}
}
}
@@ -323,8 +321,13 @@ class GooglePlatformAnalytics(private val context: Context, private val analytic
private inner class DatadogLogWriter : LogWriter() {
override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) {
val logger = datadogLogger ?: return
+ // The Datadog SDK turns any log at ERROR or above into a RUM error purely from the level β it has no
+ // per-call opt-out β so downgrading to WARN is the only way to keep an expected condition out of RUM
+ // error tracking while still emitting the log line. Note this deliberately keeps CancellationException
+ // at error here even though Crashlytics drops it; see shouldDowngradeForDatadog.
+ val effectiveSeverity = if (shouldDowngradeForDatadog(severity, throwable)) Severity.Warn else severity
val datadogPriority =
- when (severity) {
+ when (effectiveSeverity) {
Severity.Verbose -> android.util.Log.VERBOSE
Severity.Debug -> android.util.Log.DEBUG
Severity.Info -> android.util.Log.INFO
diff --git a/core/ble/build.gradle.kts b/core/ble/build.gradle.kts
index df2eeff200..3fb444e884 100644
--- a/core/ble/build.gradle.kts
+++ b/core/ble/build.gradle.kts
@@ -25,7 +25,8 @@ kotlin {
sourceSets {
commonMain.dependencies {
- implementation(projects.core.common)
+ // api: BleScanStartException implements core.common's ExpectedCondition in its public supertype list.
+ api(projects.core.common)
implementation(projects.core.di)
implementation(projects.core.model)
diff --git a/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt b/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt
index 5979a6e0c2..15ca0155a0 100644
--- a/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt
+++ b/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt
@@ -16,24 +16,55 @@
*/
package org.meshtastic.core.ble
+import org.meshtastic.core.common.log.ExpectedCondition
import kotlin.time.Duration
-/** Known reasons a BLE discovery scan failed before Android registered the scanner. */
-enum class BleScanStartFailureReason(val androidCode: String, val description: String) {
+/**
+ * Known reasons a BLE discovery scan failed before Android registered the scanner.
+ *
+ * Every reason here is an environment state β a permission not granted, a radio switched off, an OS quota β so
+ * [BleScanStartException] is an [ExpectedCondition] and is never reported as a crash.
+ *
+ * @property androidCode the platform-level code or constant this maps to, used in log lines.
+ * @property description a human-readable explanation for log lines.
+ * @property label the stable, low-cardinality [ExpectedCondition.expectedConditionLabel] for rate tracking.
+ */
+enum class BleScanStartFailureReason(val androidCode: String, val description: String, val label: String) {
ApplicationRegistrationFailed(
androidCode = "SCAN_FAILED_APPLICATION_REGISTRATION_FAILED(2)",
description = "Android could not register the app for BLE scanning",
+ label = "ble-scan-registration-failed",
),
MissingScanPermission(
androidCode = "MISSING_SCAN_PERMISSION",
description = "A runtime permission required for BLE scanning is not granted",
+ label = "ble-scan-missing-permission",
),
ScanningTooFrequently(
androidCode = "SCAN_FAILED_SCANNING_TOO_FREQUENTLY(6)",
description = "Android rejected a BLE scan because the app reached its scan-start quota",
+ label = "ble-scan-too-frequently",
+ ),
+ BluetoothDisabled(
+ androidCode = "BLUETOOTH_DISABLED",
+ description = "Bluetooth is switched off",
+ label = "ble-scan-bluetooth-disabled",
+ ),
+ LocationServicesDisabled(
+ androidCode = "LOCATION_SERVICES_DISABLED",
+ description = "Location services are off, and this Android version requires them for BLE scanning",
+ label = "ble-scan-location-services-disabled",
),
}
-/** A discovery scan-start failure. No advertisements can be delivered until a future scan starts successfully. */
+/**
+ * A discovery scan-start failure. No advertisements can be delivered until a future scan starts successfully.
+ *
+ * This is an [ExpectedCondition]: every [BleScanStartFailureReason] describes the environment refusing the scan, not a
+ * defect in the app, so callers should surface it to the user and log it at warn level rather than reporting it.
+ */
class BleScanStartException(val reason: BleScanStartFailureReason, cause: Throwable, val retryAfter: Duration? = null) :
- IllegalStateException("BLE scan could not start: ${reason.androidCode}", cause)
+ IllegalStateException("BLE scan could not start: ${reason.androidCode}", cause),
+ ExpectedCondition {
+ override val expectedConditionLabel: String = reason.label
+}
diff --git a/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt b/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt
index 4a433a758a..a96d4dafb9 100644
--- a/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt
+++ b/core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt
@@ -18,6 +18,8 @@ package org.meshtastic.core.ble
import com.juul.kable.Advertisement
import com.juul.kable.Scanner
+import com.juul.kable.UnmetRequirementException
+import com.juul.kable.UnmetRequirementReason
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.channelFlow
@@ -71,6 +73,10 @@ open class KableBleScanner(private val loggingConfig: BleLoggingConfig) : BleSca
return scanner.advertisements.map(Advertisement::toScanResult)
}
+ // ThrowsCount: three deliberate rethrow paths, one per exception family Kable can surface here β cancellation
+ // (must propagate untouched), UnmetRequirementException (an IOException) and IllegalStateException. They have no
+ // common supertype below Exception, so merging them would mean catching Exception broadly instead.
+ @Suppress("ThrowsCount")
override fun scan(timeout: Duration, serviceUuid: Uuid?, address: String?): Flow<BleDevice> {
val filter = resolveKableScanFilter(serviceUuid = serviceUuid, address = address)
@@ -91,6 +97,12 @@ open class KableBleScanner(private val loggingConfig: BleLoggingConfig) : BleSca
}
} catch (ex: CancellationException) {
throw ex
+ } catch (ex: UnmetRequirementException) {
+ // Kable models "Bluetooth is off" and "location services are off" as an IOException, so these
+ // never matched the IllegalStateException branch below and escaped raw to ViewModel-level
+ // catch-alls that log at error β the top source of Crashlytics/RUM noise. Map them to the
+ // typed, non-reported BleScanStartException instead.
+ throw ex.asBleScanStartException()
} catch (ex: IllegalStateException) {
throw ex.asBleScanStartExceptionOrNull() ?: ex
}
@@ -99,6 +111,23 @@ open class KableBleScanner(private val loggingConfig: BleLoggingConfig) : BleSca
}
}
+/**
+ * Maps Kable's typed [UnmetRequirementReason] onto the matching [BleScanStartFailureReason].
+ *
+ * Kable's reason enum is exhaustive over the preconditions it checks, so this needs no message matching β unlike the
+ * [IllegalStateException] paths below, which Kable only distinguishes by message text.
+ *
+ * Kept `internal` and separate from the exception so it stays unit-testable: [UnmetRequirementException] has an
+ * `internal` constructor in Kable and cannot be instantiated from our tests.
+ */
+internal fun UnmetRequirementReason.toBleScanStartFailureReason(): BleScanStartFailureReason = when (this) {
+ UnmetRequirementReason.BluetoothDisabled -> BleScanStartFailureReason.BluetoothDisabled
+ UnmetRequirementReason.LocationServicesDisabled -> BleScanStartFailureReason.LocationServicesDisabled
+}
+
+private fun UnmetRequirementException.asBleScanStartException(): BleScanStartException =
+ BleScanStartException(reason.toBleScanStartFailureReason(), this)
+
private fun Throwable.asBleScanStartExceptionOrNull(): BleScanStartException? {
var current: Throwable? = this
var depth = 0
diff --git a/core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/BleScanStartExceptionTest.kt b/core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/BleScanStartExceptionTest.kt
new file mode 100644
index 0000000000..4743d32557
--- /dev/null
+++ b/core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/BleScanStartExceptionTest.kt
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2026 Meshtastic LLC
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+package org.meshtastic.core.ble
+
+import co.touchlab.kermit.Severity
+import com.juul.kable.UnmetRequirementReason
+import org.meshtastic.core.common.log.isExpectedCondition
+import org.meshtastic.core.common.log.shouldReportAsException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+/**
+ * A BLE scan refused by the environment must never reach a crash reporter.
+ *
+ * Bluetooth off, location services off and a missing scan permission were four of the loudest Crashlytics issues during
+ * 2.8.0 triage, all of them non-actionable.
+ */
+class BleScanStartExceptionTest {
+
+ @Test
+ fun `every scan-start failure is an expected condition`() {
+ BleScanStartFailureReason.entries.forEach { reason ->
+ val exception = BleScanStartException(reason, cause = IllegalStateException("cause"))
+ assertTrue(exception.isExpectedCondition(), "$reason must be an expected condition")
+ assertEquals(reason.label, exception.expectedConditionLabel)
+ }
+ }
+
+ @Test
+ fun `scan-start failures are never reported even when logged at error`() {
+ BleScanStartFailureReason.entries.forEach { reason ->
+ val exception = BleScanStartException(reason, cause = IllegalStateException("cause"))
+ assertFalse(
+ shouldReportAsException(Severity.Error, exception),
+ "$reason must not be recorded as a non-fatal",
+ )
+ }
+ }
+
+ @Test
+ fun `labels are unique and low cardinality`() {
+ val labels = BleScanStartFailureReason.entries.map { it.label }
+ assertEquals(labels.size, labels.toSet().size, "labels must be unique to be usable as rate keys")
+ labels.forEach { label ->
+ assertTrue(label.isNotBlank(), "label must not be blank")
+ assertEquals(label.lowercase(), label, "label '$label' must be lowercase for stable grouping")
+ }
+ }
+
+ // Kable's UnmetRequirementException has an internal constructor, so the reason mapping is verified directly.
+ @Test
+ fun `kable unmet-requirement reasons map onto scan-start reasons`() {
+ assertEquals(
+ BleScanStartFailureReason.BluetoothDisabled,
+ UnmetRequirementReason.BluetoothDisabled.toBleScanStartFailureReason(),
+ )
+ assertEquals(
+ BleScanStartFailureReason.LocationServicesDisabled,
+ UnmetRequirementReason.LocationServicesDisabled.toBleScanStartFailureReason(),
+ )
+ }
+
+ @Test
+ fun `every kable unmet-requirement reason is mapped`() {
+ // Guards against a Kable upgrade adding a reason that silently falls through to error-level logging.
+ UnmetRequirementReason.entries.forEach { it.toBleScanStartFailureReason() }
+ }
+}
diff --git a/core/common/build.gradle.kts b/core/common/build.gradle.kts
index 36c0b4b474..edc318faff 100644
--- a/core/common/build.gradle.kts
+++ b/core/common/build.gradle.kts
@@ -31,7 +31,8 @@ kotlin {
api(libs.kotlinx.datetime)
api(libs.okio)
api(libs.uri.kmp)
- implementation(libs.kermit)
+ // api: `shouldReportAsException` exposes Kermit's Severity in its signature.
+ api(libs.kermit)
}
androidMain.dependencies { api(libs.androidx.core.ktx) }
}
diff --git a/core/common/src/commonMain/kotlin/org/meshtastic/core/common/log/ExpectedCondition.kt b/core/common/src/commonMain/kotlin/org/meshtastic/core/common/log/ExpectedCondition.kt
new file mode 100644
index 0000000000..a8c47077e4
--- /dev/null
+++ b/core/common/src/commonMain/kotlin/org/meshtastic/core/common/log/ExpectedCondition.kt
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2026 Meshtastic LLC
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+package org.meshtastic.core.common.log
+
+import co.touchlab.kermit.Severity
+import kotlinx.coroutines.CancellationException
+
+/**
+ * Marks a [Throwable] as an *expected condition*: a state the app is designed to encounter and recover from, caused by
+ * the environment rather than by a defect in this code.
+ *
+ * Bluetooth switched off, a runtime permission the user has not granted, location services disabled, and a deliberate
+ * disconnect are all expected conditions. They deserve a log line β and are worth watching as a *rate* β but they are
+ * not bugs anyone can act on, so they must never be recorded as exceptions in Crashlytics or Datadog RUM. Reporting
+ * them buries genuine regressions during release triage.
+ *
+ * Implement this on exception types whose very existence means "the environment said no". Where an exception type is
+ * shared between expected and genuine failures, keep the type clean and simply log the expected call site at
+ * [Severity.Warn] instead. An [ExpectedCondition] is suppressed by both sinks β see [shouldReportAsException]
+ * (Crashlytics) and [shouldDowngradeForDatadog] (Datadog), which agree here but deliberately differ on cancellation.
+ */
+interface ExpectedCondition {
+ /**
+ * Stable, low-cardinality label naming which condition occurred, e.g. `ble-bluetooth-disabled`. This is what makes
+ * the condition countable as a rate in the log backend instead of an error, so keep it free of addresses, node ids,
+ * timings and any other per-user detail.
+ */
+ val expectedConditionLabel: String
+}
+
+/** Cause chains are walked with a depth cap so a malformed or self-referential chain cannot spin. */
+private const val MAX_CAUSE_DEPTH = 10
+
+/**
+ * Returns the [ExpectedCondition.expectedConditionLabel] of the first [ExpectedCondition] in this throwable's cause
+ * chain, or `null` when nothing in the chain is an expected condition.
+ *
+ * The chain is walked because transport and coroutine machinery routinely wraps the original cause.
+ */
+fun Throwable.expectedConditionLabel(): String? {
+ var current: Throwable? = this
+ var depth = 0
+ while (current != null && depth < MAX_CAUSE_DEPTH) {
+ (current as? ExpectedCondition)?.let {
+ return it.expectedConditionLabel
+ }
+ current = current.cause
+ depth++
+ }
+ return null
+}
+
+/** Returns `true` when this throwable, or anything in its cause chain, is an [ExpectedCondition]. */
+fun Throwable.isExpectedCondition(): Boolean = expectedConditionLabel() != null
+
+/**
+ * Whether a log line should become a **Crashlytics** non-fatal.
+ *
+ * A log below [Severity.Error] is never reported by either sink, so warn-level logging remains the simplest way to
+ * record an expected condition without reporting it.
+ *
+ * The two sinks deliberately differ on [CancellationException] β see [shouldDowngradeForDatadog].
+ */
+fun shouldReportAsException(severity: Severity, throwable: Throwable?): Boolean = when {
+ severity < Severity.Error -> false
+
+ // Top-level only, matching the long-standing Crashlytics behaviour. Deliberately NOT a cause-chain walk:
+ // coroutine machinery routinely attaches a cancellation as the cause of an unrelated genuine failure, and
+ // unwrapping here would silently drop those reports. An ExpectedCondition marker is an explicit statement by
+ // the author about the nature of the condition, so that one is safe to unwrap; an incidental cancellation
+ // buried in a cause chain is not.
+ throwable is CancellationException -> false
+
+ throwable?.isExpectedCondition() == true -> false
+
+ else -> true
+}
+
+/**
+ * Whether the **Datadog** writer must downgrade an error-level log to `WARN`.
+ *
+ * Datadog has no per-call opt-out: its SDK turns *any* log at `ERROR` or above into a RUM error, throwable or not.
+ * Downgrading the emitted level is the only way to keep something out of RUM error tracking while still emitting the
+ * log line.
+ *
+ * This intentionally does **not** mirror [shouldReportAsException] for [CancellationException]. Crashlytics filters
+ * cancellations because it is a crash-triage tool; Datadog keeps them because a cancellation logged at error level
+ * means some call site *swallowed* it instead of rethrowing β broken structured concurrency, and a real bug. That
+ * asymmetry is what surfaced the swallowed-cancellation defects fixed in #6468, so it is load-bearing: do not "unify"
+ * the two rules.
+ */
+fun shouldDowngradeForDatadog(severity: Severity, throwable: Throwable?): Boolean =
+ severity >= Severity.Error && throwable?.isExpectedCondition() == true
diff --git a/core/common/src/commonTest/kotlin/org/meshtastic/core/common/log/ExpectedConditionTest.kt b/core/common/src/commonTest/kotlin/org/meshtastic/core/common/log/ExpectedConditionTest.kt
new file mode 100644
index 0000000000..e3f74cbc72
--- /dev/null
+++ b/core/common/src/commonTest/kotlin/org/meshtastic/core/common/log/ExpectedConditionTest.kt
@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2026 Meshtastic LLC
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ */
+package org.meshtastic.core.common.log
+
+import co.touchlab.kermit.Severity
+import kotlinx.coroutines.CancellationException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+/**
+ * Locks down the reporting contract that both Crashlytics and Datadog RUM consult.
+ *
+ * Regressions here are silent and expensive: they either bury real crashes under expected-condition noise, or stop a
+ * genuine defect from being reported at all.
+ */
+class ExpectedConditionTest {
+
+ private class TestExpected(override val expectedConditionLabel: String = "test-expected") :
+ IllegalStateException("expected"),
+ ExpectedCondition
+
+ // ββ shouldReportAsException ββββββββββββββββββββββββββββββββββββββββββββββββββββββ
+
+ @Test
+ fun `severities below Error are never reported`() {
+ val severities = listOf(Severity.Verbose, Severity.Debug, Severity.Info, Severity.Warn)
+ severities.forEach { severity ->
+ assertFalse(
+ shouldReportAsException(severity, RuntimeException("boom")),
+ "$severity with a throwable must not be reported",
+ )
+ assertFalse(shouldReportAsException(severity, null), "$severity without a throwable must not be reported")
+ }
+ }
+
+ @Test
+ fun `genuine errors are still reported`() {
+ assertTrue(shouldReportAsException(Severity.Error, RuntimeException("boom")))
+ assertTrue(shouldReportAsException(Severity.Assert, RuntimeException("boom")))
+ }
+
+ @Test
+ fun `error without a throwable is still reported`() {
+ // Crashlytics synthesises an Exception(message) for these, so the gate must stay open.
+ assertTrue(shouldReportAsException(Severity.Error, null))
+ }
+
+ @Test
+ fun `cancellation is never reported`() {
+ assertFalse(shouldReportAsException(Severity.Error, CancellationException("cancelled")))
+ assertFalse(shouldReportAsException(Severity.Assert, CancellationException("cancelled")))
+ }
+
+ @Test
+ fun `expected conditions are never reported even at error severity`() {
+ assertFalse(shouldReportAsException(Severity.Error, TestExpected()))
+ assertFalse(shouldReportAsException(Severity.Assert, TestExpected()))
+ }
+
+ @Test
+ fun `expected condition wrapped in a plain exception is not reported`() {
+ val wrapped = RuntimeException("transport failed", TestExpected())
+ assertFalse(shouldReportAsException(Severity.Error, wrapped))
+ }
+
+ @Test
+ fun `a genuine failure carrying a cancellation cause is still reported`() {
+ // Coroutine machinery routinely attaches a cancellation as the cause of an unrelated real failure.
+ // Unwrapping the chain for CancellationException would silently drop these reports.
+ val wrapped = RuntimeException("write failed", CancellationException("scope closed"))
+ assertTrue(shouldReportAsException(Severity.Error, wrapped))
+ }
+
+ // ββ Datadog downgrade rule (deliberately differs on cancellation) ββββββββββββββββ
+
+ @Test
+ fun `datadog downgrades expected conditions`() {
+ assertTrue(shouldDowngradeForDatadog(Severity.Error, TestExpected()))
+ assertTrue(shouldDowngradeForDatadog(Severity.Assert, RuntimeException("outer", TestExpected())))
+ }
+
+ @Test
+ fun `datadog keeps cancellation at error even though crashlytics drops it`() {
+ // Load-bearing asymmetry: a cancellation logged at error means a call site swallowed it instead of
+ // rethrowing β broken structured concurrency, and a real bug. This is the signal that surfaced #6468.
+ val cancellation = CancellationException("cancelled")
+ assertFalse(shouldReportAsException(Severity.Error, cancellation))
+ assertFalse(shouldDowngradeForDatadog(Severity.Error, cancellation))
+ }
+
+ @Test
+ fun `datadog leaves genuine errors and sub-error severities alone`() {
+ assertFalse(shouldDowngradeForDatadog(Severity.Error, RuntimeException("boom")))
+ assertFalse(shouldDowngradeForDatadog(Severity.Error, null))
+ assertFalse(shouldDowngradeForDatadog(Severity.Warn, TestExpected()))
+ }
+
+ // ββ cause-chain walking ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ
+
+ @Test
+ fun `label is read from the throwable itself`() {
+ assertEquals("test-expected", TestExpected().expectedConditionLabel())
+ assertTrue(TestExpected().isExpectedCondition())
+ }
+
+ @Test
+ fun `label is read through the cause chain`() {
+ val wrapped = RuntimeException("outer", IllegalStateException("middle", TestExpected("ble-bluetooth-disabled")))
+ assertEquals("ble-bluetooth-disabled", wrapped.expectedConditionLabel())
+ assertTrue(wrapped.isExpectedCondition())
+ }
+
+ @Test
+ fun `plain throwable has no label`() {
+ val plain = RuntimeException("outer", IllegalStateException("inner"))
+ assertNull(plain.expectedConditionLabel())
+ assertFalse(plain.isExpectedCondition())
+ }
+
+ @Test
+ fun `cause chain walking is depth limited`() {
+ // A chain longer than the cap must terminate rather than spin, even though the marker is out of reach.
+ var deep: Throwable = TestExpected()
+ repeat(50) { deep = RuntimeException("layer", deep) }
+ assertNull(deep.expectedConditionLabel())
+ assertFalse(deep.isExpectedCondition())
+ }
+
+ @Test
+ fun `self referential cause chain terminates`() {
+ val selfReferencing =
+ object : RuntimeException("loop") {
+ override val cause: Throwable
+ get() = this
+ }
+ assertNull(selfReferencing.expectedConditionLabel())
+ }
+}
diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt
index 814cf39fa0..59e426e5ef 100644
--- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt
+++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImpl.kt
@@ -620,7 +620,11 @@ class MeshConnectionManagerImpl(
if (serviceRepository.connectionState.value !is ConnectionState.Connecting) {
return@handledLaunch
}
- Logger.e {
+ // Warn, not error: the watchdog firing is the recovery mechanism working, and the cause is a
+ // stalled radio or link rather than a defect here. A throwable-less Logger.e still synthesises a
+ // non-fatal in Crashlytics and a RUM error, which made this one of the loudest issues in triage.
+ // Track it as a rate over this log line instead.
+ Logger.w {
"Fast-handshake watchdog expired after progress stalled β requesting forced transport restart"
}
runSiblingHandshakeRecovery()
diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt
index ada456a58e..83c01c6b1e 100644
--- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt
+++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/repository/MQTTRepositoryImpl.kt
@@ -31,6 +31,7 @@ import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.Semaphore
import kotlinx.coroutines.sync.withPermit
+import kotlinx.io.IOException
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
@@ -208,7 +209,19 @@ class MQTTRepositoryImpl(
}
else -> {
- Logger.e(result.exceptionOrNull()) { "MQTT connect failed, retrying in ${reconnectDelay}ms" }
+ val failure = result.exceptionOrNull()
+ // Broker- and network-side failures are what this retry loop exists to absorb β an
+ // unreachable host, a TLS problem, a dropped connection, or a broker that violates the
+ // MQTT 5 spec (e.g. the topic-alias limit). None are defects in this app, and reporting
+ // every retry as a non-fatal drowned real regressions.
+ //
+ // Anything else landing here is unexpected β a fault in our own connect/subscribe setup
+ // rather than the peer's β so it keeps reporting.
+ if (failure.isExpectedMqttRetryFailure()) {
+ Logger.w(failure) { "MQTT connect failed, retrying in ${reconnectDelay}ms" }
+ } else {
+ Logger.e(failure) { "MQTT connect failed unexpectedly, retrying in ${reconnectDelay}ms" }
+ }
delay(reconnectDelay)
reconnectDelay =
(reconnectDelay * RECONNECT_BACKOFF_MULTIPLIER).coerceAtMost(MAX_RECONNECT_DELAY_MS)
@@ -307,6 +320,24 @@ class MQTTRepositoryImpl(
}
}
+/**
+ * `true` when an MQTT connect/subscribe failure is one the retry loop is designed to absorb, rather than a defect worth
+ * reporting to Crashlytics/Datadog.
+ *
+ * Covers the MQTT client's own sealed error hierarchy β a lost connection, and protocol violations by the broker such
+ * as an inbound topic alias above the advertised maximum β plus transport-level I/O failures (unreachable host, TLS,
+ * socket reset). All are peer- or network-side and not actionable by this app.
+ *
+ * Deliberately narrow: an unexpected exception escaping our own client/state setup is a real bug and must keep
+ * reporting, so anything outside these families returns `false`.
+ */
+private fun Throwable?.isExpectedMqttRetryFailure(): Boolean = when (this) {
+ null -> false
+ is MqttException -> true
+ is IOException -> true
+ else -> false
+}
+
internal data class MqttClientSetup(
val ownerId: String,
val mqttConfig: ModuleConfig.MQTTConfig?,
diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml
index fea80c3548..ab1e581230 100644
--- a/core/resources/src/commonMain/composeResources/values/strings.xml
+++ b/core/resources/src/commonMain/composeResources/values/strings.xml
@@ -149,6 +149,7 @@
<string name="bluetooth_feature_discovery">Discovery</string>
<string name="bluetooth_feature_discovery_description">Find and identify Meshtastic devices near you.</string>
<string name="bluetooth_permission">Bluetooth</string>
+ <string name="bluetooth_scan_location_services_disabled">Location services are off. Turn them on to scan for nearby devices.</string>
<string name="bluetooth_scan_missing_permission">Bluetooth scan needs permission. Grant the Nearby devices (or Location) permission to find devices.</string>
<string name="bluetooth_scan_start_failed">Bluetooth scan couldn't start. Try again, or toggle Bluetooth if the problem continues.</string>
<plurals name="bluetooth_scan_too_frequent">
diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/ViewModelExtensions.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/ViewModelExtensions.kt
index b27e7391d3..eee65e6ca1 100644
--- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/ViewModelExtensions.kt
+++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/ViewModelExtensions.kt
@@ -33,6 +33,7 @@ import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
+import org.meshtastic.core.common.log.expectedConditionLabel
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.UiText
import org.meshtastic.core.resources.unknown_error
@@ -122,7 +123,14 @@ fun safeLaunch(
throw e
} catch (e: Exception) {
val label = tag ?: "safeLaunch"
- Logger.e(e) { "[$label] Unhandled exception" }
+ // Expected conditions (Bluetooth off, permission not granted, β¦) still reach the user as an error
+ // event, but are logged at warn so neither Crashlytics nor Datadog RUM records them as a defect.
+ val expectedLabel = e.expectedConditionLabel()
+ if (expectedLabel != null) {
+ Logger.w(e) { "[$label] Expected condition: $expectedLabel" }
+ } else {
+ Logger.e(e) { "[$label] Unhandled exception" }
+ }
val message = e.message?.let { UiText.DynamicString(it) } ?: UiText.Resource(Res.string.unknown_error)
errorEvents?.tryEmit(message)
}
diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt
index 3d64a55473..898524ebe0 100644
--- a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt
+++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt
@@ -59,6 +59,8 @@ import org.meshtastic.core.repository.RadioPrefs
import org.meshtastic.core.repository.ServiceRepository
import org.meshtastic.core.repository.UiPrefs
import org.meshtastic.core.resources.Res
+import org.meshtastic.core.resources.bluetooth_disabled
+import org.meshtastic.core.resources.bluetooth_scan_location_services_disabled
import org.meshtastic.core.resources.bluetooth_scan_missing_permission
import org.meshtastic.core.resources.bluetooth_scan_start_failed
import org.meshtastic.core.resources.bluetooth_scan_too_frequent
@@ -76,8 +78,49 @@ internal val BLE_SCAN_START_FAILURE_RETRY_COOLDOWN = 15.seconds
private const val BLE_SCAN_START_FAILURE_MESSAGE_FALLBACK =
"Bluetooth scan couldn't start. Try again, or toggle Bluetooth if the problem continues."
-private fun effectiveBleScanRetryCooldown(retryAfter: Duration?): Duration =
- maxOf(BLE_SCAN_START_FAILURE_RETRY_COOLDOWN, retryAfter ?: Duration.ZERO)
+/**
+ * How long to block scan restarts after a scan-start failure.
+ *
+ * A cooldown only helps where retrying too soon is itself the problem β Android's scan-start quota, or a registration
+ * failure that needs time to settle. [BleScanStartFailureReason.BluetoothDisabled] and
+ * [BleScanStartFailureReason.LocationServicesDisabled] instead clear the moment the user flips a system toggle, so
+ * holding the scan button dead for 15s after they have fixed it would just look broken.
+ */
+private fun effectiveBleScanRetryCooldown(reason: BleScanStartFailureReason, retryAfter: Duration?): Duration =
+ when (reason) {
+ BleScanStartFailureReason.BluetoothDisabled,
+ BleScanStartFailureReason.LocationServicesDisabled,
+ -> retryAfter ?: Duration.ZERO
+
+ BleScanStartFailureReason.ApplicationRegistrationFailed,
+ BleScanStartFailureReason.MissingScanPermission,
+ BleScanStartFailureReason.ScanningTooFrequently,
+ ->
+ maxOf(BLE_SCAN_START_FAILURE_RETRY_COOLDOWN, retryAfter ?: Duration.ZERO)
+ }
+
+/**
+ * Last-resort English copy used only when compose-resources cannot resolve the translated string.
+ *
+ * Kept in step with the `bluetooth_scan_*` resources so the degraded path still names the actual problem instead of
+ * falling back to generic "scan couldn't start" advice for every reason.
+ */
+private fun untranslatedScanStartFailureMessage(reason: BleScanStartFailureReason, retryCooldownSeconds: Long): String =
+ when (reason) {
+ BleScanStartFailureReason.ScanningTooFrequently -> {
+ val unit = if (retryCooldownSeconds == 1L) "second" else "seconds"
+ "Bluetooth scan limit reached. Try again in $retryCooldownSeconds $unit."
+ }
+
+ BleScanStartFailureReason.BluetoothDisabled -> "Bluetooth is off. Turn it on to scan for nearby devices."
+
+ BleScanStartFailureReason.LocationServicesDisabled ->
+ "Location services are off. Turn them on to scan for nearby devices."
+
+ BleScanStartFailureReason.ApplicationRegistrationFailed,
+ BleScanStartFailureReason.MissingScanPermission,
+ -> BLE_SCAN_START_FAILURE_MESSAGE_FALLBACK
+ }
private fun Duration.roundedUpWholeSeconds(): Long {
val completeSeconds = inWholeSeconds
@@ -403,8 +446,8 @@ open class ScannerViewModel(
scanJob = null
_isBleScanning.value = false
uiPrefs.setBleAutoScan(false)
- val retryCooldown = effectiveBleScanRetryCooldown(exception.retryAfter)
- startBleScanRetryCooldown(retryCooldown)
+ val retryCooldown = effectiveBleScanRetryCooldown(exception.reason, exception.retryAfter)
+ if (retryCooldown > Duration.ZERO) startBleScanRetryCooldown(retryCooldown)
Logger.w(exception) {
"BLE scan could not start: ${exception.reason.androidCode} (${exception.reason.description})"
@@ -425,16 +468,14 @@ open class ScannerViewModel(
retryCooldownSeconds.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(),
retryCooldownSeconds,
)
+
+ BleScanStartFailureReason.BluetoothDisabled -> getStringSuspend(Res.string.bluetooth_disabled)
+
+ BleScanStartFailureReason.LocationServicesDisabled ->
+ getStringSuspend(Res.string.bluetooth_scan_location_services_disabled)
}
}
- .getOrDefault(
- if (exception.reason == BleScanStartFailureReason.ScanningTooFrequently) {
- val unit = if (retryCooldownSeconds == 1L) "second" else "seconds"
- "Bluetooth scan limit reached. Try again in $retryCooldownSeconds $unit."
- } else {
- BLE_SCAN_START_FAILURE_MESSAGE_FALLBACK
- },
- )
+ .getOrDefault(untranslatedScanStartFailureMessage(exception.reason, retryCooldownSeconds))
serviceRepository.setErrorMessage(text = errorMessage, severity = Severity.Warn)
}
diff --git a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
index 2e222cdc2a..44db8cf3a5 100644
--- a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
+++ b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
@@ -152,6 +152,53 @@ class ScannerViewModelTest {
assertEquals(2, scanAttempts)
}
+ @Test
+ fun `bluetooth-disabled failure allows an immediate retry once the user re-enables it`() = runTest {
+ // No cooldown for preconditions the user clears with a system toggle β a dead scan button right after they
+ // switched Bluetooth back on reads as a broken app.
+ var scanAttempts = 0
+ every { bleScanner.scan(any(), any()) } returns
+ flow {
+ scanAttempts += 1
+ throw BleScanStartException(
+ reason = BleScanStartFailureReason.BluetoothDisabled,
+ cause = IllegalStateException("Bluetooth disabled"),
+ )
+ }
+
+ viewModel.startBleScan()
+ assertEquals(1, scanAttempts)
+ assertEquals(false, viewModel.isBleScanning.value)
+ assertEquals("Bluetooth is off. Turn it on to scan for nearby devices.", serviceRepository.errorMessage.value)
+
+ // Immediately retryable β no waiting on the scheduler.
+ viewModel.startBleScan()
+ assertEquals(2, scanAttempts)
+ }
+
+ @Test
+ fun `location-services-disabled failure allows an immediate retry`() = runTest {
+ var scanAttempts = 0
+ every { bleScanner.scan(any(), any()) } returns
+ flow {
+ scanAttempts += 1
+ throw BleScanStartException(
+ reason = BleScanStartFailureReason.LocationServicesDisabled,
+ cause = IllegalStateException("Location services are required for scanning but are disabled"),
+ )
+ }
+
+ viewModel.startBleScan()
+ assertEquals(1, scanAttempts)
+ assertEquals(
+ "Location services are off. Turn them on to scan for nearby devices.",
+ serviceRepository.errorMessage.value,
+ )
+
+ viewModel.startBleScan()
+ assertEquals(2, scanAttempts)
+ }
+
@Test
fun `scan quota failure honors retry-after cooldown`() = runTest {
var scanAttempts = 0
Served by rngit 1.4.2 - Generated in 0.34s